Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

動けるようにする #1288

Open
wants to merge 24 commits into
base: fix/openapi
Choose a base branch
from
Open

動けるようにする #1288

wants to merge 24 commits into from

Conversation

Eraxyso
Copy link

@Eraxyso Eraxyso commented Dec 19, 2024

Closes #1261

controller testに関する部分はあと #1271 でやります

変更箇所リスト:

  • wire(これに伴いcontroller/middleware -> handler/middleware)
  • OapiRequestValidatorを削除(Authenticationに関する部分がうまくいけないので)
  • openapiのlocal server urlの更新
  • CI workflowの更新(バージョンのアップデート、setup-goのcacheとactoins/cacheの機能が重複するのでsetup-goのcacheをオフにした...)
  • model test & lintが通れるようにいろいろな修正(一部使わない変数、関数とかがコメントアウトされた...)

@Eraxyso Eraxyso marked this pull request as ready for review January 14, 2025 06:17
@Eraxyso Eraxyso requested a review from kaitoyama January 14, 2025 06:17
@kaitoyama
Copy link
Contributor

@Eraxyso ありがとうございます!
結構色々修正してもらうことになってしまったようなので、どういう意図の変更をしたのかのリストを書いて欲しいです

  • 〇〇がうまく行くように◇◇を△△に

みたいな感じでお願いしたいです

@Eraxyso
Copy link
Author

Eraxyso commented Jan 16, 2025

↑prの説明に追記しました

Copy link
Contributor

@kaitoyama kaitoyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

基本的に良さそうです、少しだけ確認したいことがあったのでコメントしました。

userSet = userSet.Difference(groupUserSet)
return userSet.ToSlice(), groups, nil
}
// func minimizeUsersAndGroups(users []string, groups []uuid.UUID) ([]string, []uuid.UUID, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

多分これ、目的はusersをgroupsに含まれないものだけにする関数で、usersとgroupsを保存/更新するときに叩く想定で作ってそうだけど、rolloutだけあれば問題なさそうであれば、コメントアウトじゃなくて削除でもいいかも(gitなので)

description: 回答したもの(answered)か未回答のもの(unanswered)かを選別
schema:
$ref: "#/components/schemas/AnsweredType"
# answeredInQuery:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この辺は使ってなかったからコメントアウトした感じ?

args: args{
userID: questionnairesTestUserID,
sort: "",
search: "",
pageNum: 100000,
pageNum: 1,
onlyTargetingMe: true,
onlyAdministratedByMe: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

すごい直感に反する命名なので、後でschemaから変えた方が良いかも。今はこれでOK

自分が管理者になっていないもののみ取得 (true), 管理者になっているものも含めてすべて取得 (false)。デフォルトはfalse。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants